Support non-unique keys for dictionary column types by davidwendt · Pull Request #22839 · NVIDIA/cudf

davidwendt · 2026-06-10T17:38:02Z

Description

Updates libcudf DICTIONARY columns to support non-unique keys.
Duplicate keys are support on input for all libcudf APIs but any APIs that return dictionary columns will likely have non-unique keys. The exception to this is the make_dictionary_column which does not inspect the keys, indices it is given and the cudf::dictionary::set_keys() which always honors the given keys in uniqueness (or not) and order.

Checklist

I am familiar with the Contributing Guidelines.
New or existing tests cover these changes.
The documentation is up to date with these changes.

copy-pr-bot · 2026-06-10T17:38:06Z

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

davidwendt · 2026-06-10T17:39:42Z

/ok to test

davidwendt · 2026-06-12T07:41:07Z

/ok to test

pmattione-nvidia · 2026-06-29T16:07:09Z

are there any existing kernels that assume dictionaries have unique keys?

davidwendt · 2026-06-29T16:09:47Z

are there any existing kernels that assume dictionaries have unique keys?

I've not found any as of yet. Still working on this.

davidwendt · 2026-07-01T18:34:21Z

/ok to test

davidwendt · 2026-07-01T18:34:42Z

@coderabbitai full review

coderabbitai · 2026-07-01T18:34:47Z

✅ Action performed

Full review finished.

coderabbitai · 2026-07-01T18:41:48Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

📝 Walkthrough

Walkthrough

Dictionary documentation now allows duplicate keys. Argmin/argmax and minmax reductions now operate on dictionary keys directly via lazy iterators and keys-type dispatch. Tests were added for duplicate-key behavior across dictionary APIs and reductions.

Changes

Dictionary duplicate-keys support

Layer / File(s)	Summary
Documentation updates `cpp/include/cudf/dictionary/dictionary_column_view.hpp`, `cpp/include/cudf/dictionary/dictionary_factories.hpp`, `cpp/include/cudf/dictionary/update_keys.hpp`, `cpp/doxygen/developer_guide/DEVELOPER_GUIDE.md`	Dictionary docs now describe keys as non-unique, remove prior uniqueness and ordering assumptions, and add a note about duplicate keys in `set_keys`.
Dictionary argmin/argmax iterator logic `cpp/src/reductions/argmax.cu`, `cpp/src/reductions/argmin.cu`, `cpp/src/reductions/extrema_utils.cuh`	Argmin and argmax now dispatch on dictionary key types, and extrema utilities compute dictionary extrema directly from lazy iterators with null-aware handling.
Dictionary minmax reduction rework `cpp/src/reductions/minmax.cu`	Minmax reductions now dispatch on dictionary key types, reduce dictionary keys through lazy iterators, and return scalar results through a dictionary-specific path.
Dictionary API duplicate-key tests `cpp/tests/dictionary/add_keys_test.cpp`, `cpp/tests/dictionary/factories_test.cpp`, `cpp/tests/dictionary/gather_test.cpp`, `cpp/tests/dictionary/remove_keys_test.cpp`, `cpp/tests/dictionary/set_keys_test.cpp`, `cpp/tests/dictionary/sort_test.cpp`	Duplicate-key tests were added for add_keys, factories, gather, remove_keys, set_keys, and sort, with decoded-output checks and key-count assertions where applicable.
Dictionary reduction tests with unsorted duplicate keys `cpp/tests/reductions/reduction_tests.cpp`	Reduction tests now validate MIN/MAX, `cudf::minmax`, and ARGMIN/ARGMAX for dictionary keys that are unsorted and duplicated.

Estimated code review effort: 4 (Complex) | ~60 minutes

Suggested labels: feature request
Suggested reviewers: bdice, vyasr, mroeschke

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the main change: allowing non-unique dictionary keys.
Description check	✅ Passed	The description is directly related to the dictionary non-unique key support changes.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands.}

mhaseeb123

Approving with minor nits

mhaseeb123 · 2026-07-29T20:52:25Z

+    auto minimum        = make_fixed_width_scalar(key_type, stream, mr);
+    auto maximum        = make_fixed_width_scalar(key_type, stream, mr);


Claude flagged that make_fixed_width_scalar(data_type, ...) goes through scalar_construction_helper, which constructs with is_valid = false.

Also ref: Copilot's concern about decimal scale being dropped seems unhandled

Suggested change

auto minimum = make_fixed_width_scalar(key_type, stream, mr);

auto maximum = make_fixed_width_scalar(key_type, stream, mr);

auto make_scalar = [&]() -> std::unique_ptr<scalar> {

if constexpr (cudf::is_fixed_point<T>()) {

return make_fixed_point_scalar<T>(

storage_type{}, numeric::scale_type{key_type.scale()}, stream, mr);

} else {

auto scalar = make_fixed_width_scalar(key_type, stream, mr);

scalar->set_valid_async(true, stream);

return std::move(scalar);

}

};

auto minimum = make_scalar();

auto maximum = make_scalar();

Please also assert validity in the new tests — DictionaryReductionTest.MinMaxUnsortedDuplicateKeys only compares .first / ->value(), and no pre-existing gtest checks is_valid() for a fixed-width dictionary min/max, which is why CI is green on this. DictionaryTypes is also only {int32_t, int64_t, float, double}, so a decimal case would be needed to pin the scale.

The decimal scale being dropped is a separate issue in this source code (not just dictionary) which I will address in a follow-on PR. The is_valid case is valid and I'll fix that here.

mhaseeb123 · 2026-07-29T21:08:06Z

+  auto d_dictionary = column_device_view::create(col, stream);
+  if (col.has_nulls()) {
+    auto pair_to_minmax = cuda::make_transform_iterator(
+      cudf::dictionary::detail::make_dictionary_pair_iterator<T>(*d_dictionary, true),


Follow-up PR: Perhaps we can we add a benchmark, if none exists, to measure whether loading keys[indices[i]] per row rather than reducing indices and slicing keys affects low-cardinality dictionary minmax performance?

Yes, some exploration would be good here. This could first do a groupby-count to determine which keys are present (possibly including null values) and then do the minmax reduction over the keys with nonzero counts.

bdice

Approving with minor comments, but we need the null test cases and other pieces that @mhaseeb123 mentioned.

bdice · 2026-07-29T21:46:02Z

+ * Although duplicate keys are allowed, indices in the returned dictionary may
+ * only reference one of the duplicates.


I read this 3 times before I started to understand it.

What is guaranteed/preserved here? I think the number of keys in the output dictionary is keys.size(), even if keys contains duplicates. But because we have to reassign all the indices to match the new keys, we arbitrarily choose one of those duplicate values to provide the index for reassignment. Do we (or should we) provide any guarantee around which of the duplicates is used for the index? First/last/any?

It is hard to provide determinism and lossless conversion/transformation with the ambiguities introduced by duplicate keys.

Maybe there's a better way to say this, or maybe the current state is fine.

We introduced non-determinism by no longer requiring sorted keys.
So determinism would be a big requirement and we would have reconsider all of the current dictionary implementation in libcudf again.

bdice · 2026-07-29T22:34:18Z

+  auto d_dictionary = column_device_view::create(col, stream);
+  if (col.has_nulls()) {
+    auto pair_to_minmax = cuda::make_transform_iterator(
+      cudf::dictionary::detail::make_dictionary_pair_iterator<T>(*d_dictionary, true),


Yes, some exploration would be good here. This could first do a groupby-count to determine which keys are present (possibly including null values) and then do the minmax reduction over the keys with nonzero counts.

davidwendt · 2026-07-31T13:57:30Z

/merge

Support non-unique keys for dictionary column types

adb5e88

davidwendt self-assigned this Jun 10, 2026

davidwendt added 2 - In Progress Currently a work in progress libcudf Affects libcudf (C++/CUDA) code. improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Jun 10, 2026

Merge branch 'main' into dict-non-unique-keys

b681491

davidwendt added 6 commits June 22, 2026 08:35

Merge branch 'main' into dict-non-unique-keys

f525d92

Merge branch 'main' into dict-non-unique-keys

3c7bb7e

add more gtests

20a011b

Merge branch 'main' into dict-non-unique-keys

14c1296

Merge branch 'main' into dict-non-unique-keys

c5ab27b

Merge branch 'main' into dict-non-unique-keys

9bce5c6

mhaseeb123 mentioned this pull request Jun 30, 2026

Transcode parquet to cuDF dictionaries for flat STRING columns #22532

Merged

3 tasks

davidwendt added 5 commits June 30, 2026 16:57

Merge branch 'main' into dict-non-unique-keys

1217031

Merge branch 'main' into dict-non-unique-keys

1c7f8cd

Merge branch 'main' into dict-non-unique-keys

ed286ad

Merge branch 'main' into dict-non-unique-keys

ff49295

fix min/max logic for non-unique keys

b5c0f64

davidwendt added 3 - Ready for Review Ready for review by team and removed 2 - In Progress Currently a work in progress labels Jul 1, 2026

davidwendt added 3 commits July 14, 2026 14:47

Merge branch 'main' into dict-non-unique-keys

e71a5bb

Merge branch 'main' into dict-non-unique-keys

e9d45e5

Merge branch 'main' into dict-non-unique-keys

b83ece0

davidwendt mentioned this pull request Jul 15, 2026

Add cudf::dictionary::remove_duplicate_keys API #23275

Closed

3 tasks

davidwendt added 2 commits July 15, 2026 14:37

Merge branch 'main' into dict-non-unique-keys

b604531

fix minmax for decimal scale

f16be6d

GregoryKimball moved this from Burndown to Slip in libcudf Jul 20, 2026

GregoryKimball moved this from Slip to Burndown in libcudf Jul 20, 2026

davidwendt moved this from Burndown to Slip in libcudf Jul 21, 2026

davidwendt added 4 commits July 22, 2026 08:52

Merge branch 'main' into dict-non-unique-keys

410e759

Merge branch 'main' into dict-non-unique-keys

de5e51b

Merge branch 'main' into dict-non-unique-keys

d5a7508

Merge branch 'main' into dict-non-unique-keys

60d17d3

mhaseeb123 approved these changes Jul 29, 2026

View reviewed changes

Comment thread cpp/src/reductions/minmax.cu Outdated

Comment thread cpp/src/reductions/minmax.cu Outdated

davidwendt added 2 commits July 29, 2026 16:22

Merge branch 'main' into dict-non-unique-keys

d165781

change thrust::transform_iterator to cuda::transform_iterator

b686565

mhaseeb123 reviewed Jul 29, 2026

View reviewed changes

bdice approved these changes Jul 29, 2026

View reviewed changes

davidwendt added 2 commits July 30, 2026 08:58

Merge branch 'main' into dict-non-unique-keys

cfd4dfa

add null gtests; fix is-valid; add dispatch-type parm

9e952d6

rapids-bot Bot merged commit 0470761 into NVIDIA:main Jul 31, 2026
134 of 136 checks passed

davidwendt deleted the dict-non-unique-keys branch July 31, 2026 13:57

coderabbitai Bot mentioned this pull request Aug 4, 2026

Use dictionary indices in join functions #23534

Merged

3 tasks

josephine-wolf-oberholtzer added this to libcudf Aug 12, 2026

josephine-wolf-oberholtzer moved this to Slip in libcudf Aug 12, 2026

GregoryKimball moved this from Slip to Landed in libcudf Aug 20, 2026

GregoryKimball removed this from libcudf Aug 21, 2026

GregoryKimball moved this from Slip to Landed in libcudf Aug 21, 2026

		auto minimum = make_fixed_width_scalar(key_type, stream, mr);
		auto maximum = make_fixed_width_scalar(key_type, stream, mr);

-    auto minimum        = make_fixed_width_scalar(key_type, stream, mr);
-    auto maximum        = make_fixed_width_scalar(key_type, stream, mr);
+    auto make_scalar = [&]() -> std::unique_ptr<scalar> {
+      if constexpr (cudf::is_fixed_point<T>()) {
+        return make_fixed_point_scalar<T>(
+          storage_type{}, numeric::scale_type{key_type.scale()}, stream, mr);
+      } else {
+        auto scalar = make_fixed_width_scalar(key_type, stream, mr);
+        scalar->set_valid_async(true, stream);
+        return std::move(scalar);
+      }
+    };
+    auto minimum = make_scalar();
+    auto maximum = make_scalar();

		* Although duplicate keys are allowed, indices in the returned dictionary may
		* only reference one of the duplicates.

Uh oh!

Conversation

davidwendt commented Jun 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

copy-pr-bot Bot commented Jun 10, 2026

Uh oh!

davidwendt commented Jun 10, 2026

Uh oh!

davidwendt commented Jun 12, 2026

Uh oh!

pmattione-nvidia commented Jun 29, 2026

Uh oh!

davidwendt commented Jun 29, 2026

Uh oh!

davidwendt commented Jul 1, 2026

Uh oh!

davidwendt commented Jul 1, 2026

Uh oh!

coderabbitai Bot commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Uh oh!

mhaseeb123 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mhaseeb123 Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davidwendt Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mhaseeb123 Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bdice Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bdice left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bdice Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

davidwendt Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

bdice Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

davidwendt commented Jul 31, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

davidwendt commented Jun 10, 2026 •

edited

Loading

coderabbitai Bot commented Jul 1, 2026 •

edited

Loading

coderabbitai Bot commented Jul 1, 2026 •

edited

Loading

mhaseeb123 Jul 29, 2026 •

edited

Loading

mhaseeb123 Jul 29, 2026 •

edited

Loading

bdice left a comment •

edited

Loading